Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

converted Jazzicon component to functional component and added story #15638

Merged
merged 10 commits into from
Sep 8, 2022
Merged

converted Jazzicon component to functional component and added story #15638

merged 10 commits into from
Sep 8, 2022

Conversation

NidhiKJha
Copy link
Member

@NidhiKJha NidhiKJha commented Aug 18, 2022 •
edited
Loading

Explanation

Currently, the Jazzicon component is a class-based component and doesn't have a story

Since we are gonna utilise this component in other IA/Nav components, this PR is to add a story to Jazzicon while converting it to functional component

Screenshots/Screencaps

After

Screenshot 2022-08-18 at 8 45 51 PM

Manual Testing Steps

  • Go to the latest storybook build of this PR
  • Search Jazzicon

Pre-Merge Checklist

  • PR template is filled out
  • [x] IF this PR fixes a bug, a test that would have caught the bug has been added N/A
  • [x] PR is linked to the appropriate GitHub issue N/A
  • [x] PR has been added to the appropriate release Milestone N/A

+ If there are functional changes:

  • Manual testing complete & passed
  • [ ] "Extension QA Board" label has been applied N/A

@NidhiKJha NidhiKJha requested a review from a team as a code owner August 18, 2022 15:17
@NidhiKJha NidhiKJha added the team-design-system All issues relating to design system in Extension label Aug 18, 2022
@georgewrmarshall georgewrmarshall added the area-UI Relating to the user interface. label Aug 18, 2022
@metamaskbot
Copy link
Collaborator

Builds ready [2306003]
Page Load Metrics (1739 ± 113 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint86147106178
domContentLoaded157125371727232111
load157125521739234113
domInteractive157125371727232111

highlights:

storybook

@NidhiKJha NidhiKJha force-pushed the jazzicon-stories branch 2 times, most recently from fdeca45 to e90491d Compare August 18, 2022 17:59
@metamaskbot
Copy link
Collaborator

Builds ready [e90491d]
Page Load Metrics (1709 ± 46 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint91148113136
domContentLoaded1577187416938641
load1577195817099546
domInteractive1577187416938641

highlights:

storybook

@metamaskbot
Copy link
Collaborator

Builds ready [2b7472f]
Page Load Metrics (1723 ± 29 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint94157112178
domContentLoaded1607180417025828
load1626186817236029
domInteractive1607180417025828

highlights:

storybook

georgewrmarshall
georgewrmarshall requested changes Aug 22, 2022
View reviewed changes
Copy link
Contributor

@georgewrmarshall georgewrmarshall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good! Some updates

ui/components/ui/jazzicon/README.mdx Outdated Show resolved Hide resolved
ui/components/ui/jazzicon/jazzicon.component.js Outdated Show resolved Hide resolved
ui/components/ui/jazzicon/jazzicon.component.js Outdated Show resolved Hide resolved
@NidhiKJha NidhiKJha requested a review from brad-decker August 23, 2022 08:23
georgewrmarshall
georgewrmarshall reviewed Aug 23, 2022
View reviewed changes
Copy link
Contributor

@georgewrmarshall georgewrmarshall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Would just get a second opinion on the use of dangerouslySetInnerHTML

@NidhiKJha
Copy link
Member Author

Looks good! Would just get a second opinion on the use of dangerouslySetInnerHTML

cc @darkwing @brad-decker

@NidhiKJha NidhiKJha requested a review from darkwing August 24, 2022 14:23
@metamaskbot
Copy link
Collaborator

Builds ready [c2f0d0f]
Page Load Metrics (1827 ± 48 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint961709277463222
domContentLoaded1627199118109043
load16492061182710048
domInteractive1627199118109043

highlights:

storybook

@metamaskbot
Copy link
Collaborator

Builds ready [37fe3f5]
Page Load Metrics (2044 ± 137 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint942775271575276
domContentLoaded162727812002272131
load162727812044285137
domInteractive162727812002272131

highlights:

storybook

@metamaskbot
Copy link
Collaborator

Builds ready [043c091]
Page Load Metrics (1699 ± 46 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint921642187334161
domContentLoaded1582202016819043
load1582202016999546
domInteractive1582202016819043

highlights:

storybook

@NidhiKJha NidhiKJha requested a review from brad-decker August 26, 2022 17:05
@metamaskbot
Copy link
Collaborator

Builds ready [687b8bd]
Page Load Metrics (1981 ± 56 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1072148234439211
domContentLoaded17912247194613062
load17912247198111656
domInteractive17912247194613062

highlights:

storybook

georgewrmarshall
georgewrmarshall reviewed Sep 6, 2022
View reviewed changes
Copy link
Contributor

@georgewrmarshall georgewrmarshall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like prop table is broken. If you can't fix you can just add manually with a mark down table :)

Screen Shot 2022-09-06 at 9 44 30 AM

@metamaskbot
Copy link
Collaborator

Builds ready [e0f941a]
Page Load Metrics (1825 ± 90 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint911611212110
domContentLoaded16132257180917383
load16362257182518790
domInteractive16132257180917383

highlights:

storybook

georgewrmarshall
georgewrmarshall approved these changes Sep 7, 2022
View reviewed changes
Copy link
Contributor

@georgewrmarshall georgewrmarshall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh it's the optional chaining issue again. Will approve this and hopefully issue is fixed in updated version of storybook

@NidhiKJha NidhiKJha merged commit 838cd5b into develop Sep 8, 2022
@NidhiKJha NidhiKJha deleted the jazzicon-stories branch September 8, 2022 09:29
@github-actions github-actions bot locked and limited conversation to collaborators Sep 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@georgewrmarshall georgewrmarshall georgewrmarshall approved these changes

@brad-decker brad-decker brad-decker approved these changes

@darkwing darkwing darkwing approved these changes

@garrettbear garrettbear Awaiting requested review from garrettbear

Assignees

@NidhiKJha NidhiKJha

Labels
area-UI Relating to the user interface. team-design-system All issues relating to design system in Extension
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@NidhiKJha @metamaskbot @darkwing @brad-decker @georgewrmarshall